Fix mypy errors #925
Fix mypy errors #925
Conversation
WalkthroughReplace direct dict indexing with safe Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant HA as Home Assistant
participant Entity as Plugwise Entity
participant Device as Device Data (dict)
HA->>Entity: Read property (is_on / native_value / temperature / options)
Entity->>Device: device.get(primary_key, default) → nested .get(...)
alt Key present & valid
Device-->>Entity: value
Entity-->>HA: value (may be Optional typed)
else Key missing or invalid
Device-->>Entity: default (None / [] / fallback)
Entity-->>HA: None / [] / fallback
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
custom_components/plugwise/climate.py (1)
199-210: Bug: comparing string mode to list[HVACMode] always falls back to HEAT.
mode not in self.hvac_modescompares a str toHVACModemembers, so it’s always True. Fix by normalizing toHVACModefirst.- if ( - mode := self.device.get(CLIMATE_MODE) - ) is None or mode not in self.hvac_modes: # pw-beta add to Core - return HVACMode.HEAT # pragma: no cover - # pw-beta homekit emulation - if self._homekit_enabled and self._homekit_mode == HVACMode.OFF: - mode = HVACMode.OFF # pragma: no cover - - return HVACMode(mode) + mode = self.device.get(CLIMATE_MODE) + if mode is None: + return HVACMode.HEAT # pragma: no cover + try: + hvac = HVACMode(mode) + except ValueError: + return HVACMode.HEAT # pragma: no cover + if hvac not in self.hvac_modes: + return HVACMode.HEAT # pragma: no cover + # pw-beta homekit emulation + if self._homekit_enabled and self._homekit_mode == HVACMode.OFF: + return HVACMode.OFF # pragma: no cover + return hvac
🧹 Nitpick comments (8)
custom_components/plugwise/entity.py (1)
80-88: Simplify AVAILABLE check for readability (same behavior).Equivalent, slightly clearer, and keeps mypy happy:
- and (AVAILABLE not in self.device or self.device.get(AVAILABLE) is True) + and (self.device.get(AVAILABLE, True) is True)custom_components/plugwise/binary_sensor.py (1)
167-178: Tighten return typing to Optional[bool] to avoid Any leaking.If mypy is in strict mode, consider an explicit cast:
- return self.device.get(BINARY_SENSORS, {}).get(self.entity_description.key) + value = self.device.get(BINARY_SENSORS, {}).get(self.entity_description.key) + return cast(bool | None, value)Add import near existing typing imports:
-from typing import Any +from typing import Any, castcustom_components/plugwise/sensor.py (1)
518-522: Constrain native_value to expected numeric types.Optional cast to satisfy strict mypy:
- return self.device.get(SENSORS, {}).get(self.entity_description.key) # Upstream consts + value = self.device.get(SENSORS, {}).get(self.entity_description.key) # Upstream consts + return cast(int | float | None, value)And add:
-from dataclasses import dataclass +from dataclasses import dataclass +from typing import castcustom_components/plugwise/select.py (1)
135-143: Minor: normalize annotation spacing and tighten types for mypy.
- Use spaced PEP 604 annotation for consistency.
- Cast on dict access to avoid Any.
- def current_option(self) -> str| None: + def current_option(self) -> str | None: @@ - return self.device.get(self.entity_description.key) + value = self.device.get(self.entity_description.key) + return cast(str | None, value) @@ - return self.device.get(self.entity_description.options_key, []) + value = self.device.get(self.entity_description.options_key, []) + return cast(list[str], value)Add:
-from homeassistant.components.select import SelectEntity, SelectEntityDescription +from homeassistant.components.select import SelectEntity, SelectEntityDescription +from typing import castcustom_components/plugwise/switch.py (1)
129-133: Constrain return type to Optional[bool] (avoid Any).- return self.device.get(SWITCHES, {}).get(self.entity_description.key) # Upstream const + value = self.device.get(SWITCHES, {}).get(self.entity_description.key) # Upstream const + return cast(bool | None, value)And at top:
-from typing import Any +from typing import Any, castcustom_components/plugwise/number.py (2)
124-136: Remove unused attribute and DRY nested gets.
self.actuatorisn’t used elsewhere; drop to avoid confusion.- Optional: store
ctrl = self.device.get(description.key, {})to avoid repeating.- self.actuator = self.device.get(description.key) # Upstream self.device_id = device_id @@ - self._attr_native_max_value = self.device.get(description.key, {}).get(UPPER_BOUND, 100.0) # Upstream const - self._attr_native_min_value = self.device.get(description.key, {}).get(LOWER_BOUND, 0.0) # Upstream const + ctrl = self.device.get(description.key, {}) + self._attr_native_max_value = ctrl.get(UPPER_BOUND, 100.0) # Upstream const + self._attr_native_min_value = ctrl.get(LOWER_BOUND, 0.0) # Upstream const @@ - native_step = self.device.get(description.key, {}).get(RESOLUTION, 0.5) # Upstream const + native_step = ctrl.get(RESOLUTION, 0.5) # Upstream const
138-141: Align return type with Optional[float] explicitly.- return self.device.get(self.entity_description.key, {}).get("setpoint") + value = self.device.get(self.entity_description.key, {}).get("setpoint") + return cast(float | None, value)Add if not present:
+from typing import castcustom_components/plugwise/climate.py (1)
211-235: Guard SELECT_REGULATION_MODE to avoid KeyError.
REGULATION_MODES in _gateway_datadoesn’t guaranteeSELECT_REGULATION_MODEexists.- if REGULATION_MODES in self._gateway_data: - if self._gateway_data[SELECT_REGULATION_MODE] == HVACAction.COOLING: + if REGULATION_MODES in self._gateway_data: + selected = self._gateway_data.get(SELECT_REGULATION_MODE) + if selected == HVACAction.COOLING: hvac_modes.append(HVACMode.COOL) - if self._gateway_data[SELECT_REGULATION_MODE] == HVACAction.HEATING: + if selected == HVACAction.HEATING: hvac_modes.append(HVACMode.HEAT)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
custom_components/plugwise/binary_sensor.py(2 hunks)custom_components/plugwise/climate.py(6 hunks)custom_components/plugwise/entity.py(1 hunks)custom_components/plugwise/number.py(1 hunks)custom_components/plugwise/select.py(1 hunks)custom_components/plugwise/sensor.py(1 hunks)custom_components/plugwise/switch.py(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
custom_components/plugwise/select.py (1)
custom_components/plugwise/entity.py (1)
device(91-93)
custom_components/plugwise/switch.py (2)
custom_components/plugwise/binary_sensor.py (1)
is_on(168-177)custom_components/plugwise/entity.py (1)
device(91-93)
custom_components/plugwise/binary_sensor.py (2)
custom_components/plugwise/switch.py (1)
is_on(130-132)custom_components/plugwise/entity.py (1)
device(91-93)
custom_components/plugwise/sensor.py (2)
custom_components/plugwise/number.py (1)
native_value(138-140)custom_components/plugwise/entity.py (1)
device(91-93)
custom_components/plugwise/climate.py (1)
custom_components/plugwise/entity.py (1)
device(91-93)
custom_components/plugwise/number.py (2)
custom_components/plugwise/entity.py (1)
device(91-93)custom_components/plugwise/sensor.py (1)
native_value(519-521)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Prepare
🔇 Additional comments (1)
custom_components/plugwise/climate.py (1)
129-134: LGTM: safer access + Optional returns.Using
.get(..., default)for bounds/steps and returningfloat | Nonefor temperatures/preset access matches HA patterns and avoids KeyError.Please confirm HA Core tests for these entities run clean with these Optional returns (no assumptions about non-None downstream).
Also applies to: 169-172, 174-181, 183-189, 191-197, 296-299
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
custom_components/plugwise/number.py (2)
128-131: Cast bounds to float to satisfy mypy and avoid type driftctrl.get(...) is Any; assigning to _attr_native_max_value/_attr_native_min_value (float | None) can still trip mypy. Cast to float to make the type explicit.
Apply:
- ctrl = self.device.get(description.key, {}) - self._attr_native_max_value = ctrl.get(UPPER_BOUND, 100.0) # Upstream const - self._attr_native_min_value = ctrl.get(LOWER_BOUND, 0.0) # Upstream const + ctrl = self.device.get(description.key, {}) + self._attr_native_max_value = float(ctrl.get(UPPER_BOUND, 100.0)) # Upstream const + self._attr_native_min_value = float(ctrl.get(LOWER_BOUND, 0.0)) # Upstream const
132-136: Ensure step is a float before clampingSame mypy concern for native_step. Casting also guards against accidental non‑float values.
- native_step = ctrl.get(RESOLUTION, 0.5) # Upstream const + native_step = float(ctrl.get(RESOLUTION, 0.5)) # Upstream const if description.key != TEMPERATURE_OFFSET: # Upstream const native_step = max(native_step, 0.5) self._attr_native_step = native_stepcustom_components/plugwise/climate.py (2)
129-134: Cast thermostat limits/step to float to quiet mypyget(...) returns Any; explicit float(...) avoids Optional[Any] flowing into attr* fields.
- self._attr_max_temp = min(self.device.get(THERMOSTAT, {}).get(UPPER_BOUND, 35.0), 35.0) - self._attr_min_temp = self.device.get(THERMOSTAT, {}).get(LOWER_BOUND, 0.0) + self._attr_max_temp = min(float(self.device.get(THERMOSTAT, {}).get(UPPER_BOUND, 35.0)), 35.0) + self._attr_min_temp = float(self.device.get(THERMOSTAT, {}).get(LOWER_BOUND, 0.0)) # Ensure we don't drop below 0.1 - self._attr_target_temperature_step = max( - self.device.get(THERMOSTAT, {}).get(RESOLUTION, 0.5), 0.1 - ) + self._attr_target_temperature_step = max( + float(self.device.get(THERMOSTAT, {}).get(RESOLUTION, 0.5)), 0.1 + )
150-153: Always set preset_modes attribute for consistencyIf preset_modes is an empty list, the feature remains disabled (good), but _attr_preset_modes stays unset. Setting it explicitly to None or [] improves clarity.
- if presets := self.device.get("preset_modes", None): # can be NONE + presets = self.device.get("preset_modes", None) # can be None or [] + if presets: self._attr_supported_features |= ClimateEntityFeature.PRESET_MODE - self._attr_preset_modes = presets + self._attr_preset_modes = presets
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
custom_components/plugwise/climate.py(6 hunks)custom_components/plugwise/entity.py(1 hunks)custom_components/plugwise/number.py(1 hunks)custom_components/plugwise/select.py(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- custom_components/plugwise/entity.py
- custom_components/plugwise/select.py
🧰 Additional context used
🧬 Code graph analysis (2)
custom_components/plugwise/number.py (2)
custom_components/plugwise/entity.py (1)
device(91-93)custom_components/plugwise/sensor.py (1)
native_value(519-521)
custom_components/plugwise/climate.py (2)
custom_components/plugwise/entity.py (1)
device(91-93)tests/components/plugwise/conftest.py (1)
cooling_present(49-54)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Setup for HA-core (release/master)
- GitHub Check: Prepare and validate pre-commit
🔇 Additional comments (5)
custom_components/plugwise/number.py (1)
138-141: LGTM: Optional-safe native_valueReturning Optional aligns with NumberEntity expectations and avoids KeyError.
custom_components/plugwise/climate.py (4)
169-172: LGTM: Optional-safe sensor accessAvoids KeyError; Optional return matches ClimateEntity expectations.
174-197: LGTM: Optional-safe target temperaturesAll target temperature properties now return Optional[float]; aligns with HA typing and prevents KeyError.
225-235: LGTM: Safer hvac_modes derivationUsing .get(...) for AVAILABLE_SCHEDULES and SELECT_REGULATION_MODE avoids KeyError and keeps types narrow.
301-302: LGTM: Safe preset check in HomeKit emulationGuarded ACTIVE_PRESET access avoids KeyError.
|
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
custom_components/plugwise/climate.py (1)
288-295: Pass Plugwise regulation-mode strings to set_regulation_mode (not HA enums/actions)set_regulation_mode expects Plugwise regulation-mode strings (e.g. "heating", "cooling", "off"); do not pass HVACAction objects or raw HA enum objects.
- Location: custom_components/plugwise/climate.py lines 288-295 (also check 296-304).
- Fix: convert/map before calling (e.g. set_regulation_mode(hvac_mode.value) or map HVACMode/HVACAction → Plugwise string); ensure self._previous_mode is a regulation-mode string, not an HVACAction.
🧹 Nitpick comments (3)
custom_components/plugwise/climate.py (3)
150-153: Preset handling is safe; optionally set explicit NoneSetting features only when presets exist is fine. Optionally set _attr_preset_modes = None in the else-path for explicitness (not required).
201-214: HVAC mode coercion hardened correctlyCoercion via HVACMode(mode) with fallbacks is good. Minor: caching self.hvac_modes in a local var avoids recomputation (optional).
225-239: Edge case: hvac_modes may omit both HEAT/COOL when selected is unknownIf cooling_present is True, REGULATION_MODES exists, but SELECT_REGULATION_MODE is missing/unknown, neither HEAT nor COOL is added. Consider exposing both to avoid an empty set and to align with hvac_mode’s HEAT fallback.
Apply:
- if REGULATION_MODES in self._gateway_data: - selected = self._gateway_data.get(SELECT_REGULATION_MODE) - if selected == HVACAction.COOLING: - hvac_modes.append(HVACMode.COOL) - if selected == HVACAction.HEATING: - hvac_modes.append(HVACMode.HEAT) + if REGULATION_MODES in self._gateway_data: + selected = self._gateway_data.get(SELECT_REGULATION_MODE) + if selected == HVACAction.COOLING: + hvac_modes.append(HVACMode.COOL) + elif selected == HVACAction.HEATING: + hvac_modes.append(HVACMode.HEAT) + else: + # Unknown selection: expose both to keep UI functional + hvac_modes.extend([HVACMode.HEAT, HVACMode.COOL])Also applies to: 230-234
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
custom_components/plugwise/climate.py(7 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
custom_components/plugwise/climate.py (1)
custom_components/plugwise/entity.py (1)
device(91-93)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Setup for HA-core (release/master)
🔇 Additional comments (7)
custom_components/plugwise/climate.py (7)
109-111: Type fix for _homekit_mode is correct; align _previous_mode typing tooGood change to HVACMode | None. To tighten types further, annotate _previous_mode as HVACAction (it holds HVACAction values).
Apply:
- _previous_mode: str = HVACAction.HEATING # Upstream + _previous_mode: HVACAction = HVACAction.HEATING # Upstream
129-134: Bounds/step guarded lookups LGTMUsing .get with sane defaults addresses mypy and runtime KeyError risks.
169-172: Optional current_temperature return type is appropriateGuarded access + Optional return matches HA expectations.
174-181: Optional target_temperature LGTM
183-189: Optional target_temperature_high LGTM
191-196: Optional target_temperature_low LGTM
256-257: preset_mode getter uses safe access; good
In Core testing some mypy errors show up for the Plugwise integration, fixing them first in Plugwise-beta.
Summary by CodeRabbit
Bug Fixes
Refactor